Skip to content

Optimize Base#relevant_file? #1062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Optimize Base#relevant_file? #1062

wants to merge 1 commit into from

Conversation

@fatkodima
Copy link

@fatkodima fatkodima commented Oct 26, 2020

I identified the problem, fixed and then discovered #967. So this is another implementation, with possibly the smallest change possible.

So, I profiled rubocops memory usage while running on itself:

bundle exec bin/rubocop-profile --memory lib/rubocop/cop

And get the following:

Before

Total allocated: 1.14 GB (16969980 objects)
Total retained:  7.35 MB (54861 objects)

allocated memory by gem
-----------------------------------
 408.41 MB  rubocop/lib
 297.48 MB  parser-2.7.2.0
 195.80 MB  rubocop-ast-1.0.1
 112.66 MB  rubocop-rspec-1.43.2
.....
allocated memory by file
-----------------------------------
 125.46 MB  /Users/fatkodima/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/gems/parser-2.7.2.0/lib/parser/source/tree_rewriter.rb
 110.64 MB  /Users/fatkodima/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/gems/rubocop-rspec-1.43.2/lib/rubocop/cop/rspec/base.rb
....

After

Total allocated: 1.01 GB (15940558 objects)
Total retained:  6.92 MB (49414 objects)

allocated memory by gem
-----------------------------------
 409.73 MB  rubocop/lib
 300.32 MB  parser-2.7.2.0
 197.03 MB  rubocop-ast-1.0.1
  29.16 MB  other
  23.92 MB  rubocop-performance-1.8.1
  20.42 MB  set
   6.15 MB  ast-2.4.1
   5.84 MB  regexp_parser-1.8.2
   5.22 MB  unicode-display_width-1.7.0
   4.11 MB  psych
   2.24 MB  rubocop-rspec/lib
   2.17 MB  erb
....
allocated memory by file
-----------------------------------
 125.46 MB  /Users/fatkodima/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/gems/parser-2.7.2.0/lib/parser/source/tree_rewriter.rb
  60.59 MB  /Users/fatkodima/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/gems/rubocop-ast-1.0.1/lib/rubocop/ast/node.rb
....
 204.78 kB  /Users/fatkodima/Desktop/oss/rubocop-rspec/lib/rubocop/cop/rspec/base.rb
....

And measured the impact of just rubocop-rspec.

bundle exec bin/rubocop-profile --memory --only RSpec lib/rubocop/cop

Before

Total allocated: 354.84 MB (4610105 objects)
Total retained:  5.84 MB (42878 objects)
....
allocated memory by file
-----------------------------------
 101.90 MB  /Users/fatkodima/Desktop/oss/rubocop-rspec/lib/rubocop/cop/rspec/base.rb
  40.35 MB  /Users/fatkodima/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/gems/parser-2.7.2.0/lib/parser/lexer.rb
....

After

Total allocated: 253.12 MB (3946086 objects)
Total retained:  5.86 MB (43001 objects)
....
allocated memory by file
-----------------------------------
  40.35 MB  /Users/fatkodima/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/gems/parser-2.7.2.0/lib/parser/lexer.rb
  29.81 MB  /Users/fatkodima/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/gems/rubocop-ast-1.0.1/lib/rubocop/ast/node.rb
....
 43.97 kB  /Users/fatkodima/Desktop/oss/rubocop-rspec/lib/rubocop/cop/rspec/base.rb
....

So, ~30% reducing in memory.

@pirj
Copy link
Member

pirj commented Oct 26, 2020

Thanks a lot!
We've been looking to improve #967, so probably will just merge your solution instead.

There's a semi-related #1044, but the problem there is that cops from the same department should have different Patterns

@pirj pirj changed the base branch from master to release-2.0 October 26, 2020 15:42
This was referenced Oct 26, 2020
Merged
Merged
@bquorning
Copy link
Collaborator

bquorning commented Oct 29, 2020

Fixed in #1063.

@bquorning bquorning closed this Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fatkodima @pirj @bquorning